-
Notifications
You must be signed in to change notification settings - Fork 111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add liquidity cap parameter to ZRC20 creation #3353
base: develop
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a liquidity cap parameter for ZRC20 token creation across multiple modules and components of the ZetaChain blockchain. The changes enable specifying a customizable liquidity limit during the deployment and whitelisting of tokens, enhancing the flexibility of token management. The modifications span CLI interfaces, message structures, keeper implementations, and test cases to comprehensively integrate this new functionality. Changes
Possibly related issues
Possibly related PRs
Suggested reviewers
The implementation elegantly resolves the operational complexity of managing token liquidity by integrating a single, comprehensive parameter across the system's components. Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3353 +/- ##
===========================================
+ Coverage 62.43% 62.44% +0.01%
===========================================
Files 449 449
Lines 31706 31719 +13
===========================================
+ Hits 19795 19808 +13
Misses 11024 11024
Partials 887 887
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🧹 Nitpick comments (14)
x/crosschain/client/cli/cli_whitelist_erc20.go (1)
18-18
: Enhance command description with parameter format details.Add details about the expected format and constraints of the
liquidityCap
parameter in the command description.- Use: "whitelist-erc20 [erc20Address] [chainID] [name] [symbol] [decimals] [gasLimit] [liquidityCap]", + Use: "whitelist-erc20 [erc20Address] [chainID] [name] [symbol] [decimals] [gasLimit] [liquidityCap (uint)]", + Long: "Add a new erc20 token to whitelist. The liquidityCap parameter specifies the maximum allowed liquidity in base units.",x/fungible/client/cli/tx_deploy_fungible_coin_zrc_4.go (1)
18-18
: Enhance command description with parameter format details.Add details about the expected format and constraints of the
liquidityCap
parameter in the command description.- Use: "deploy-fungible-coin-zrc-4 [erc-20] [foreign-chain] [decimals] [name] [symbol] [coin-type] [gas-limit] [liquidity-cap]", + Use: "deploy-fungible-coin-zrc-4 [erc-20] [foreign-chain] [decimals] [name] [symbol] [coin-type] [gas-limit] [liquidity-cap (uint)]", + Long: "Deploy a fungible coin ZRC4 contract. The liquidity-cap parameter specifies the maximum allowed liquidity in base units.",e2e/e2etests/test_solana_whitelist_spl.go (2)
44-44
: Improve readability of the liquidity cap value.The hardcoded value would be more maintainable and readable if defined as a constant with a descriptive name.
+const ( + // TestLiquidityCap represents 100M tokens with 18 decimals + TestLiquidityCap = "100000000000000000000000000" +) - sdkmath.NewUintFromString("100000000000000000000000000"), + sdkmath.NewUintFromString(TestLiquidityCap),
42-45
: Consider adding test cases with different liquidity cap values.The test would be more comprehensive with additional test cases that verify the behavior with different liquidity cap values, including edge cases.
Consider adding test cases for:
- Minimum acceptable value
- Maximum value that can be handled
- Zero value (should fail)
x/fungible/types/message_deploy_fungible_coin_zrc20_test.go (1)
68-81
: Enhance test coverage for liquidity cap validation.While the nil check test case is good, consider adding test cases for:
- Zero liquidity cap
- Maximum allowed liquidity cap
- Negative values (if applicable to the type)
{ name: "zero liquidity cap", msg: types.NewMsgDeployFungibleCoinZRC20( sample.AccAddress(), "test erc20", 1, 6, "test", "test", coin.CoinType_ERC20, 10, sdkmath.ZeroUint(), ), err: sdkerrors.ErrInvalidRequest, }, { name: "excessive liquidity cap", msg: types.NewMsgDeployFungibleCoinZRC20( sample.AccAddress(), "test erc20", 1, 6, "test", "test", coin.CoinType_ERC20, 10, sdkmath.NewUintFromString("115792089237316195423570985008687907853269984665640564039457584007913129639935"), ), err: sdkerrors.ErrInvalidRequest, },e2e/e2etests/test_whitelist_erc20.go (1)
44-44
: Consider documenting the liquidity cap value.The large value (100000000000000000000000000) would benefit from:
- A named constant with documentation explaining the choice
- Verification that the cap was correctly set after whitelisting
+// DefaultTestLiquidityCap represents 100M tokens with 18 decimals +const DefaultTestLiquidityCap = "100000000000000000000000000" -sdkmath.NewUintFromString("100000000000000000000000000"), +sdkmath.NewUintFromString(DefaultTestLiquidityCap),Additionally, consider adding verification:
// Verify liquidity cap was set correctly cap, err := erc20ZRC20.LiquidityCap(&bind.CallOpts{}) require.NoError(r, err) require.Equal(r, DefaultTestLiquidityCap, cap.String())x/crosschain/keeper/utils_test.go (1)
171-172
: Consider parameterizing the liquidity cap in test helpers.Both
setupGasCoin
anddeployZRC20
use a hardcoded value of 1000 for the liquidity cap. Consider making this a parameter to enable testing different scenarios and edge cases.func setupGasCoin( t *testing.T, ctx sdk.Context, k *fungiblekeeper.Keeper, evmk *evmkeeper.Keeper, chainID int64, assetName string, symbol string, + liquidityCap sdkmath.Uint, ) (zrc20 common.Address) {
Also applies to: 198-199
e2e/e2etests/test_migrate_chain_support.go (2)
83-83
: Consider using a more realistic test value for liquidity cap.The current value of "100000000000000000000000000" (1e26) might be unnecessarily large for testing purposes. Consider using a smaller, more realistic value that still validates the functionality.
- sdkmath.NewUintFromString("100000000000000000000000000"), + sdkmath.NewUintFromString("1000000000000000000"), // 1e18
170-170
: Avoid duplicating magic numbers.The liquidity cap value is duplicated. Consider extracting it to a constant for better maintainability.
+const testLiquidityCap = "1000000000000000000" // 1e18 + // ... later in the code ... - sdkmath.NewUintFromString("100000000000000000000000000"), + sdkmath.NewUintFromString(testLiquidityCap),x/crosschain/keeper/msg_server_whitelist_erc20_test.go (1)
84-84
: Extract test value to a constant.The liquidity cap value of 1000 is used repeatedly throughout the test file. Consider extracting it to a constant for better maintainability and clarity.
+const testLiquidityCap = uint64(1000) + // ... in the test ... - LiquidityCap: sdkmath.NewUint(1000), + LiquidityCap: sdkmath.NewUint(testLiquidityCap), // ... in assertions ... - uint64(1000), + testLiquidityCap,Also applies to: 99-104
docs/spec/fungible/messages.md (1)
46-46
: Fix formatting: Replace hard tab with spaces.Replace the hard tab with spaces to maintain consistent formatting throughout the documentation.
🧰 Tools
🪛 Markdownlint (0.37.0)
46-46: Column: 1
Hard tabs(MD010, no-hard-tabs)
docs/spec/crosschain/messages.md (1)
214-214
: Replace hard tab with spaces.For consistency with Markdown formatting standards, replace the hard tab with spaces.
🧰 Tools
🪛 Markdownlint (0.37.0)
214-214: Column: 1
Hard tabs(MD010, no-hard-tabs)
e2e/txserver/zeta_tx_server.go (2)
436-437
: Consider extracting the liquidity cap constant.The magic number
100000000000000000000000000
(100M * 10^18) should be extracted as a named constant at the package level for better maintainability and reusability.const ( + // DefaultLiquidityCap represents the default liquidity cap of 100M tokens with 18 decimal places + DefaultLiquidityCap = "100000000000000000000000000" ) // ZetaTxServer implementation... func (zts ZetaTxServer) DeployZRC20s(...) (*ZRC20Addresses, error) { - liquidityCap := sdkmath.NewUintFromString("100000000000000000000000000") + liquidityCap := sdkmath.NewUintFromString(DefaultLiquidityCap)
Line range hint
438-493
: Consider parameterizing the liquidity cap.The same liquidity cap is applied to all tokens regardless of their characteristics (gas tokens vs. ERC20). Consider making it configurable per token type or accepting it as a parameter in the
DeployZRC20s
function.Example refactor:
type ZRC20Deployment struct { ERC20Addr common.Address SPLAddr *solana.PublicKey + LiquidityCap sdkmath.Uint // Add liquidity cap parameter } func (zts ZetaTxServer) DeployZRC20s( zrc20Deployment ZRC20Deployment, skipChain func(chainID int64) bool, + defaultLiquidityCap sdkmath.Uint, ) (*ZRC20Addresses, error) {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
typescript/zetachain/zetacore/crosschain/tx_pb.d.ts
is excluded by!**/*_pb.d.ts
typescript/zetachain/zetacore/fungible/tx_pb.d.ts
is excluded by!**/*_pb.d.ts
x/crosschain/types/tx.pb.go
is excluded by!**/*.pb.go
,!**/*.pb.go
x/fungible/types/tx.pb.go
is excluded by!**/*.pb.go
,!**/*.pb.go
📒 Files selected for processing (30)
changelog.md
(1 hunks)docs/cli/zetacored/cli.md
(2 hunks)docs/spec/crosschain/messages.md
(1 hunks)docs/spec/fungible/messages.md
(1 hunks)e2e/e2etests/test_migrate_chain_support.go
(3 hunks)e2e/e2etests/test_solana_whitelist_spl.go
(2 hunks)e2e/e2etests/test_whitelist_erc20.go
(2 hunks)e2e/txserver/zeta_tx_server.go
(7 hunks)precompiles/bank/method_test.go
(2 hunks)precompiles/staking/staking_test.go
(1 hunks)proto/zetachain/zetacore/crosschain/tx.proto
(1 hunks)proto/zetachain/zetacore/fungible/tx.proto
(1 hunks)testutil/keeper/mocks/crosschain/fungible.go
(2 hunks)x/crosschain/client/cli/cli_whitelist_erc20.go
(4 hunks)x/crosschain/keeper/msg_server_whitelist_erc20.go
(2 hunks)x/crosschain/keeper/msg_server_whitelist_erc20_test.go
(11 hunks)x/crosschain/keeper/utils_test.go
(3 hunks)x/crosschain/types/expected_keepers.go
(2 hunks)x/crosschain/types/message_whitelist_erc20.go
(4 hunks)x/crosschain/types/message_whitelist_erc20_test.go
(8 hunks)x/fungible/client/cli/tx_deploy_fungible_coin_zrc_4.go
(4 hunks)x/fungible/keeper/evm.go
(3 hunks)x/fungible/keeper/evm_test.go
(8 hunks)x/fungible/keeper/gas_coin_and_pool.go
(3 hunks)x/fungible/keeper/gas_coin_and_pool_test.go
(12 hunks)x/fungible/keeper/msg_server_deploy_fungible_coin_zrc20.go
(2 hunks)x/fungible/keeper/msg_server_deploy_fungible_coin_zrc20_test.go
(8 hunks)x/fungible/keeper/msg_server_update_contract_bytecode_test.go
(3 hunks)x/fungible/types/message_deploy_fungible_coin_zrc20.go
(4 hunks)x/fungible/types/message_deploy_fungible_coin_zrc20_test.go
(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- changelog.md
👮 Files not reviewed due to content moderation or server errors (4)
- x/fungible/keeper/msg_server_update_contract_bytecode_test.go
- x/fungible/keeper/gas_coin_and_pool_test.go
- precompiles/staking/staking_test.go
- precompiles/bank/method_test.go
🧰 Additional context used
📓 Path-based instructions (26)
proto/zetachain/zetacore/fungible/tx.proto (1)
Pattern **/*.proto
: Review the Protobuf definitions, point out issues relative to compatibility, and expressiveness.
e2e/e2etests/test_solana_whitelist_spl.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
x/fungible/keeper/gas_coin_and_pool_test.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
proto/zetachain/zetacore/crosschain/tx.proto (1)
Pattern **/*.proto
: Review the Protobuf definitions, point out issues relative to compatibility, and expressiveness.
x/fungible/keeper/evm.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
precompiles/staking/staking_test.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
x/fungible/types/message_deploy_fungible_coin_zrc20_test.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
x/crosschain/keeper/msg_server_whitelist_erc20.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
x/fungible/keeper/gas_coin_and_pool.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
x/fungible/keeper/msg_server_update_contract_bytecode_test.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
x/crosschain/client/cli/cli_whitelist_erc20.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
x/fungible/keeper/evm_test.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/txserver/zeta_tx_server.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
x/crosschain/types/expected_keepers.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
x/crosschain/keeper/msg_server_whitelist_erc20_test.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
x/crosschain/keeper/utils_test.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
x/fungible/keeper/msg_server_deploy_fungible_coin_zrc20_test.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
x/fungible/keeper/msg_server_deploy_fungible_coin_zrc20.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
x/fungible/client/cli/tx_deploy_fungible_coin_zrc_4.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
x/crosschain/types/message_whitelist_erc20.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/e2etests/test_migrate_chain_support.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/e2etests/test_whitelist_erc20.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
x/crosschain/types/message_whitelist_erc20_test.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
precompiles/bank/method_test.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
x/fungible/types/message_deploy_fungible_coin_zrc20.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
testutil/keeper/mocks/crosschain/fungible.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
🪛 Markdownlint (0.37.0)
docs/spec/fungible/messages.md
46-46: Column: 1
Hard tabs
(MD010, no-hard-tabs)
docs/spec/crosschain/messages.md
214-214: Column: 1
Hard tabs
(MD010, no-hard-tabs)
🔇 Additional comments (15)
x/crosschain/types/message_whitelist_erc20.go (1)
Line range hint
19-36
: LGTM! Constructor parameters are well-organized.The addition of the liquidityCap parameter maintains clean code structure and follows the existing pattern.
x/fungible/keeper/msg_server_deploy_fungible_coin_zrc20.go (1)
61-61
: LGTM! Consistent parameter handling.The liquidityCap parameter is correctly propagated to both gas and non-gas coin deployment paths, maintaining consistency in the implementation.
Also applies to: 77-77
x/crosschain/types/message_whitelist_erc20_test.go (1)
92-104
: LGTM! Good test coverage for invalid liquidity cap.The test case properly validates that missing liquidity cap results in an error.
x/fungible/keeper/gas_coin_and_pool.go (1)
Line range hint
19-20
: Address FIXME comment in function documentation.The FIXME comment indicates that coin type and gas limit handling needs improvement. Consider creating a separate issue to track this technical debt.
Would you like me to create a GitHub issue to track the FIXME comment regarding proper gas limit handling based on coin type and chain?
x/fungible/keeper/msg_server_deploy_fungible_coin_zrc20_test.go (1)
88-89
: LGTM! Comprehensive test coverage for liquidity cap.The test properly verifies the liquidity cap value in the foreign coin data structure.
Also applies to: 99-104
x/crosschain/types/expected_keepers.go (1)
206-206
: LGTM! Well-defined interface change.The addition of
liquidityCap
parameter withsdkmath.Uint
type is appropriate for handling large numerical values in blockchain context.testutil/keeper/mocks/crosschain/fungible.go (1)
17-17
: LGTM! Mock implementation properly updated.The mock implementation correctly incorporates the new liquidityCap parameter, maintaining consistency with the interface changes.
Also applies to: 125-147
x/fungible/keeper/evm_test.go (1)
4-4
: LGTM! Comprehensive test coverage for liquidity cap.The test cases properly verify the liquidity cap functionality with different values and scenarios, ensuring correct behavior of the new parameter.
Also applies to: 240-240, 261-261, 287-287, 309-309, 324-324, 371-371, 386-386
docs/spec/fungible/messages.md (1)
46-46
: LGTM! Documentation updated with new field.The liquidity_cap field is properly documented in the MsgDeployFungibleCoinZRC20 message specification.
🧰 Tools
🪛 Markdownlint (0.37.0)
46-46: Column: 1
Hard tabs(MD010, no-hard-tabs)
proto/zetachain/zetacore/fungible/tx.proto (1)
72-75
: LGTM! Well-structured Protobuf field addition.The
liquidity_cap
field is properly defined with appropriate gogoproto customization for SDK's Uint type and follows sequential field numbering.proto/zetachain/zetacore/crosschain/tx.proto (1)
84-87
: LGTM! Well-structured Protobuf field addition.The
liquidity_cap
field is properly defined with appropriate gogoproto customization for SDK's Uint type and follows sequential field numbering.x/fungible/keeper/evm.go (3)
12-12
: LGTM! Appropriate import addition.The
sdkmath
import is correctly added for handling the newliquidityCap
parameter.
109-109
: LGTM! Function signature update.The
liquidityCap
parameter is properly added with the correct typesdkmath.Uint
.
167-167
: LGTM! Direct assignment of liquidity cap.The
liquidityCap
is correctly assigned to thenewCoin.LiquidityCap
field.e2e/txserver/zeta_tx_server.go (1)
Line range hint
494-507
: Verify SPL token deployment parameters.The SPL token deployment uses the same liquidity cap and gas limit as ERC20. Ensure these values are appropriate for the Solana ecosystem.
Description
Summary by CodeRabbit
Based on the comprehensive summary, here are the release notes:
Release Notes
New Features
Improvements
Technical Updates
cosmossdk.io/math
library for precise numeric handling